Skip to content

Move exception lock to ModuleInstance data.#4772

Merged
lum1n0us merged 3 commits intobytecodealliance:mainfrom
vchigrin:fine-grained-lock
Mar 16, 2026
Merged

Move exception lock to ModuleInstance data.#4772
lum1n0us merged 3 commits intobytecodealliance:mainfrom
vchigrin:fine-grained-lock

Conversation

@vchigrin
Copy link
Contributor

This lock acquired on each native function call. This cause performance impact on programs, containing many native function calls, and running in multithreaded environment.

@lum1n0us lum1n0us added the bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs. label Jan 23, 2026
@lum1n0us lum1n0us added the breaking-change Determine if this PR introduces breaking changes. It will be used by scripts to classify PRs. label Feb 3, 2026
Copy link
Contributor

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The PR has affected the layout of WASMModuleInstanceExtra, which will change key offsetof values in AOT files. Therefore, it requires a new AOT_CURRENT_VERSION in core/config.h.

  • Obviously, there is a failed CI that should be fixed.

@vchigrin vchigrin force-pushed the fine-grained-lock branch 3 times, most recently from eca416a to 3bc7eb7 Compare February 9, 2026 15:01
@vchigrin
Copy link
Contributor Author

vchigrin commented Feb 9, 2026

Thank you so much! Fixed both issues, all CI checks are green now.

@vchigrin vchigrin requested a review from lum1n0us February 9, 2026 15:22
@vchigrin vchigrin force-pushed the fine-grained-lock branch 2 times, most recently from 906cf84 to 93af779 Compare March 10, 2026 13:56
This lock acquired on each native function call. This cause performance impact
on programs, containing many native function calls, and running in multithreaded environment.
@vchigrin vchigrin force-pushed the fine-grained-lock branch from 93af779 to 2e07a8c Compare March 10, 2026 14:26
@vchigrin vchigrin force-pushed the fine-grained-lock branch from 2e07a8c to bdd43c7 Compare March 10, 2026 16:28
@vchigrin
Copy link
Contributor Author

Thank you so much for review.
Just out of curiosity, are these changes really break compatibility (and thus require AOT_CURRENT_VERSION bump)? I doubted a bit for two reasons:

  1. here are some static asserts that check offsets of some fields in AOTModuleInstanceExtra. When I started to work on this patch I thought that "preserving these offsets is all what required for binary compatibility. If these assertions did not fire, then version bump is not necessary". May be that assumption was wrong, though.
  2. One recent PR also changed layout of AOTModuleInstanceExtra, but it did not change AOT_CURRENT_VERSION.

Of course, you, as code owner, know better all nuances. I just write to ensure that we'll not force our users re-compile all their AOT programs without need.

@lum1n0us
Copy link
Contributor

The basic relationship is that WASMModuleInstanceExtraCommon common is a field within AOTModuleInstanceExtra. Subsequent fields, such as functions, will have their offsetof values changed. For protection, we tend to bump AOT_VER to let the AOT loader be aware of that.

PR#4400 was likely overlooked. Currently, the entire review system relies on manual review. Perhaps we could consider introducing other tools to assist.

@lum1n0us lum1n0us merged commit 26aa924 into bytecodealliance:main Mar 16, 2026
717 of 718 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Determine if this PR introduces breaking changes. It will be used by scripts to classify PRs. bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants